Skip to content

feat: add test sets for SSE port, protocol config, and port mapping#212

Merged
officialasishkumar merged 1 commit intomainfrom
feat/sse-port-tests
Mar 26, 2026
Merged

feat: add test sets for SSE port, protocol config, and port mapping#212
officialasishkumar merged 1 commit intomainfrom
feat/sse-port-tests

Conversation

@officialasishkumar
Copy link
Copy Markdown
Member

  • Add /ping endpoint on HTTP mux and new ping server on :8050
  • SKIP_PING_SERVER env var to disable ping server for port mapping CI tests
  • Record port-config-test: mixed HTTP (:8000) + SSE (:8047) requests
  • Record port-mapping-test: requests to :8050 for port mapping validation
  • Update keploy.yml with ssePort, protocol config, and per-test-set port mapping

- Add /ping endpoint on HTTP mux and new ping server on :8050
- SKIP_PING_SERVER env var to disable ping server for port mapping CI tests
- Record port-config-test: mixed HTTP (:8000) + SSE (:8047) requests
- Record port-mapping-test: requests to :8050 for port mapping validation
- Update keploy.yml with ssePort, protocol config, and per-test-set port mapping
Copilot AI review requested due to automatic review settings March 26, 2026 13:16
Copy link
Copy Markdown
Member

@khareyash05 khareyash05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds additional Keploy test-sets and configuration to validate multi-port behavior (HTTP + SSE) and port-mapping during CI, including a dedicated ping endpoint/server to exercise port replacement behavior.

Changes:

  • Add /ping endpoint on the HTTP mux and an optional dedicated ping server on :8050 (toggleable via SKIP_PING_SERVER).
  • Add new recorded Keploy test-sets for port-configuration (mixed HTTP + SSE) and port-mapping validation (requests to :8050).
  • Update keploy.yml with ssePort, explicit protocol port config, and per-test-set port mapping.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
http-sse/main.go Adds optional ping server on :8050 and /ping route for port-mapping validation alongside existing HTTP/SSE servers.
http-sse/keploy.yml Adds ssePort, protocol port config, and per-test-set port mapping for port-mapping-test.
http-sse/keploy/port-mapping-test/tests/test-1.yaml New recorded HTTP test case for port-mapping test-set.
http-sse/keploy/port-mapping-test/tests/test-2.yaml New recorded /ping request against :8050 for port-mapping validation.
http-sse/keploy/port-mapping-test/tests/test-3.yaml Additional recorded /ping request against :8050 for port-mapping validation.
http-sse/keploy/port-mapping-test/tests/test-4.yaml New recorded HTTP health request for port-mapping test-set.
http-sse/keploy/port-mapping-test/tests/test-5.yaml New recorded HTTP /api/hello request for port-mapping test-set.
http-sse/keploy/port-config-test/tests/test-1.yaml New recorded HTTP health request for port-config test-set.
http-sse/keploy/port-config-test/tests/test-2.yaml New recorded HTTP health request for port-config test-set.
http-sse/keploy/port-config-test/tests/test-3.yaml New recorded HTTP /api/hello request for port-config test-set.
http-sse/keploy/port-config-test/tests/test-4.yaml New recorded HTTP /api/echo request for port-config test-set.
http-sse/keploy/port-config-test/tests/test-5.yaml New recorded HTTP /api/add request for port-config test-set.
http-sse/keploy/port-config-test/tests/test-6.yaml New recorded HTTP /api/resource/:id request for port-config test-set.
http-sse/keploy/port-config-test/tests/test-7.yaml New recorded SSE student stream request against :8047 for port-config test-set.
http-sse/keploy/port-config-test/tests/test-8.yaml New recorded SSE teacher stream request against :8047 for port-config test-set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +32 to +33
// Optional ping server on :8050; disabled via SKIP_PING_SERVER=1 so that
// CI can validate replaceWith port-mapping (8050→8000) actually works.
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the ping server is disabled via SKIP_PING_SERVER=1, but the code disables it for any non-empty value (os.Getenv(...) == ""). Either update the comment to match the behavior, or change the check to only skip when the env var is explicitly set (e.g., "1").

Suggested change
// Optional ping server on :8050; disabled via SKIP_PING_SERVER=1 so that
// CI can validate replaceWith port-mapping (8050→8000) actually works.
// Optional ping server on :8050; disabled when SKIP_PING_SERVER is set
// to any non-empty value (for example, SKIP_PING_SERVER=1) so that CI
// can validate replaceWith port-mapping (8050→8000) actually works.

Copilot uses AI. Check for mistakes.
Comment on lines 158 to +203
@@ -157,6 +190,21 @@ func httpMux() http.Handler {
return mux
}

/* ----------------------------- PING :8050 ----------------------------- */

func pingMux() http.Handler {
mux := http.NewServeMux()

mux.HandleFunc("/ping", func(w http.ResponseWriter, r *http.Request) {
writeJSON(w, http.StatusOK, map[string]any{
"pong": true,
"from": "http",
})
})
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ping is implemented twice (once in httpMux and again in pingMux) with identical response bodies. To avoid accidental divergence (especially since this is used for port-mapping validation), consider extracting a shared handler function (or a small helper that writes the response) and reusing it in both muxes.

Copilot uses AI. Check for mistakes.
@officialasishkumar officialasishkumar merged commit 8f74954 into main Mar 26, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants